Skip to content

annotate with @NullMarked the whole package org.openqa.selenium.net #16494

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 2 commits into from
Nov 27, 2025
Merged

annotate with @NullMarked the whole package org.openqa.selenium.net #16494

merged 2 commits into from
Nov 27, 2025
+21 −18

Conversation

@asolntsev
Copy link
Contributor

@asolntsev asolntsev commented Oct 23, 2025 •
edited by qodo-merge-pro bot
Loading

User description

So it's not needed to duplicate @NullMarked in every file.

PR Type

Enhancement

Description

  • Consolidate @NullMarked annotation to package level

  • Remove redundant per-class @NullMarked annotations

  • Create package-info.java with package-level nullability declaration

  • Reduce code duplication across org.openqa.selenium.net package

Diagram Walkthrough

flowchart LR
  A["Individual Classes<br/>with @NullMarked"] -->|Consolidate| B["package-info.java<br/>with @NullMarked"]
  B -->|Applies to| C["All classes in<br/>org.openqa.selenium.net"]
  C -->|Result| D["Cleaner codebase<br/>No duplication"]
Loading

File Walkthrough
Relevant files
Cleanup
9 files
DefaultNetworkInterfaceProvider.java
Remove class-level @NullMarked annotation                               
+0/-2     
HostIdentifier.java
Remove class-level @NullMarked annotation                               
+0/-2     
LinuxEphemeralPortRangeDetector.java
Remove class-level @NullMarked annotation                               
+0/-2     
NetworkInterface.java
Remove class-level @NullMarked annotation                               
+0/-2     
NetworkInterfaceProvider.java
Remove interface-level @NullMarked annotation                       
+0/-2     
NetworkUtils.java
Remove class-level @NullMarked annotation                               
+0/-2     
PortProber.java
Remove class-level @NullMarked annotation                               
+0/-2     
UrlChecker.java
Remove class-level @NullMarked annotation                               
+0/-2     
Urls.java
Remove class-level @NullMarked annotation                               
+0/-2     
Enhancement
1 files
package-info.java
Create package-info with package-level @NullMarked             
+21/-0   

@asolntsev
annotation with @NullMarked the whole package org.openqa.selenium.net
edb289b 
So it's not needed to duplicate @NullMarked in every file.
@selenium-ci selenium-ci added the C-java Java Bindings label Oct 23, 2025
@qodo-merge-pro
Copy link
Contributor

qodo-merge-pro bot commented Oct 23, 2025 •
edited
Loading

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
⚪
🎫 No ticket provided 
- [ ] Create ticket/issue <!-- /create_ticket --create_ticket=true -->

</details></td></tr>
Codebase Duplication Compliance
⚪
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
⚪
No custom compliance provided

Follow the guide to enable custom compliance check.

  • Update
Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-merge-pro
Copy link
Contributor

qodo-merge-pro bot commented Oct 23, 2025 •
edited
Loading

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Learned
best practice
Ensure executor shutdown and await termination

Use try-with-resources for the InputStream (already done) and also ensure the
ExecutorService is shut down and awaited to terminate; prefer a bounded thread
pool for predictable lifecycle.

java/src/org/openqa/selenium/net/UrlChecker.java [65-104]

-ExecutorService service = Executors.newCachedThreadPool();
+ExecutorService service = Executors.newSingleThreadExecutor();
 try {
   // ...
   HttpURLConnection connection = (HttpURLConnection) url.openConnection();
   connection.setConnectTimeout((int) timeoutUnit.toMillis(timeout));
   connection.setReadTimeout((int) timeoutUnit.toMillis(timeout));
   connection.setInstanceFollowRedirects(followRedirects);
-  try (InputStream ignored =
-      connection.getInputStream()) { // Attempt to read to force a connection
+  try (InputStream ignored = connection.getInputStream()) {
     // ...
   } finally {
     connection.disconnect();
   }
   // ...
 } finally {
   service.shutdownNow();
+  service.awaitTermination(5, TimeUnit.SECONDS);
 }

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 6

__

Why:
Relevant best practice - Enforce deterministic resource cleanup with try-with-resources to close streams and executors on all paths.

Low
General
Move import before package declaration

In package-info.java, move the import statement before the annotated package
declaration to adhere to common Java conventions.

java/src/org/openqa/selenium/net/package-info.java [18-21]

+import org.jspecify.annotations.NullMarked;
+
 @NullMarked
 package org.openqa.selenium.net;
 
-import org.jspecify.annotations.NullMarked;
-
  • Apply / Chat
Suggestion importance[1-10]: 4

__

Why: The suggestion improves code style by aligning the import statement order with standard Java conventions for package-info.java files, enhancing readability and consistency with automated formatters.

Low
  • Update

@asolntsev asolntsev changed the title annotation with @NullMarked the whole package org.openqa.selenium.net annotate with @NullMarked the whole package org.openqa.selenium.net Nov 25, 2025
@asolntsev asolntsev self-assigned this Nov 27, 2025
@diemol diemol merged commit f7cc7cb into SeleniumHQ:trunk Nov 27, 2025
37 of 38 checks passed
@asolntsev asolntsev deleted the refactor/nullability-per-package branch November 27, 2025 16:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@diemol diemol diemol approved these changes

@titusfortner titusfortner Awaiting requested review from titusfortner

@joerg1985 joerg1985 Awaiting requested review from joerg1985

@iampopovich iampopovich Awaiting requested review from iampopovich

Assignees

@asolntsev asolntsev

Labels

C-java Java Bindings I-cleanup Something needs to be tidied Review effort 1/5

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@asolntsev @diemol @selenium-ci